Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(world): check namespace exists before balance transfer [L-03] #2095

Merged
merged 12 commits into from
Jan 12, 2024

Conversation

yonadaa
Copy link
Contributor

@yonadaa yonadaa commented Jan 9, 2024

No description provided.

@yonadaa yonadaa requested review from alvrs and holic as code owners January 9, 2024 13:44
Copy link

changeset-bot bot commented Jan 9, 2024

🦋 Changeset detected

Latest commit: 962af4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
@latticexyz/world Patch
@latticexyz/cli Patch
@latticexyz/dev-tools Patch
@latticexyz/store-sync Patch
@latticexyz/world-modules Patch
@latticexyz/store-indexer Patch
@latticexyz/abi-ts Patch
@latticexyz/block-logs-stream Patch
@latticexyz/common Patch
@latticexyz/config Patch
create-mud Patch
@latticexyz/ecs-browser Patch
@latticexyz/faucet Patch
@latticexyz/gas-report Patch
@latticexyz/network Patch
@latticexyz/noise Patch
@latticexyz/phaserx Patch
@latticexyz/protocol-parser Patch
@latticexyz/react Patch
@latticexyz/recs Patch
@latticexyz/schema-type Patch
@latticexyz/services Patch
@latticexyz/solecs Patch
solhint-config-mud Patch
solhint-plugin-mud Patch
@latticexyz/std-client Patch
@latticexyz/std-contracts Patch
@latticexyz/store-cache Patch
@latticexyz/store Patch
@latticexyz/utils Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -37,6 +38,9 @@ contract BalanceTransferSystem is System, IWorldErrors {
revert World_InvalidResourceType(RESOURCE_NAMESPACE, toNamespaceId, toNamespaceId.toString());
}

// Require the namespace to exist
if (!ResourceIds.getExists(toNamespaceId)) revert World_ResourceNotFound(toNamespaceId, toNamespaceId.toString());

// Require caller to have access to the namespace
AccessControl.requireAccess(fromNamespaceId, _msgSender());
Copy link
Member

@holic holic Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how much gas we'd incur if we added this check in requireAccess instead?

edit: see #2105

holic
holic previously approved these changes Jan 10, 2024
@alvrs
Copy link
Member

alvrs commented Jan 11, 2024

just added a requireExistence helper to AccessControl in #2007, we can use that for this check too!

@yonadaa
Copy link
Contributor Author

yonadaa commented Jan 11, 2024

This PR is failing integration tests because CoreSystem is too big to deploy. This should be fixed in #2046.

@yonadaa
Copy link
Contributor Author

yonadaa commented Jan 12, 2024

Using the requireExistence helper got this under the size limit!

@yonadaa yonadaa requested a review from alvrs January 12, 2024 14:12
holic
holic previously approved these changes Jan 12, 2024
.changeset/seven-carpets-develop.md Outdated Show resolved Hide resolved
@holic holic merged commit aee8020 into main Jan 12, 2024
10 of 11 checks passed
@holic holic deleted the yonadaaa/namespace-balance-transfer-lost branch January 12, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants